Skip to content

feat(build): add system CA root mode - #2324

Open
politerealism wants to merge 8 commits into
NVIDIA:mainfrom
politerealism:openshell-unbundle-certs
Open

feat(build): add system CA root mode#2324
politerealism wants to merge 8 commits into
NVIDIA:mainfrom
politerealism:openshell-unbundle-certs

Conversation

@politerealism

@politerealism politerealism commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a bundled-ca-roots Cargo feature (default on) to openshell-supervisor-network and openshell-sandbox, allowing Linux distribution builds to use the platform trust store instead of bundled Mozilla roots by building without this feature
  • Add a system-ca-roots convenience feature alias on openshell-sandbox that includes all other defaults (telemetry) except bundled CA roots, so distro builds can use --no-default-features --features system-ca-roots without manually re-adding unrelated defaults
  • Make rustls-native-certs a regular (non-optional) dependency — when bundled-ca-roots is absent, the platform trust store is used automatically
  • Switch CLI and gateway unconditionally to native CA roots (rustls-tls-native-roots, tls-rustls-ring-native-roots) so RPM-built binaries respect the system trust store without feature flags
  • Fix rust:verify:telemetry-off CI task to explicitly re-enable bundled-ca-roots when disabling default features

Related Issue

Enables downstream RPM packaging (https://gitlab.com/redhat/hummingbird/rpms/-/merge_requests/2503) to build against system CA roots.

Design

bundled-ca-roots is a Cargo feature. "System CA roots" is a build mode selected by building without bundled-ca-roots. There is no system-ca-roots feature on openshell-supervisor-network — the #[cfg(not(feature = "bundled-ca-roots"))] code path uses rustls-native-certs to read from the platform trust store.

The system-ca-roots alias on openshell-sandbox is a convenience feature that includes telemetry (and any future non-CA defaults) so distro package recipes don't need to track unrelated feature additions:

# Distro build (RPM, etc.)
cargo build -p openshell-sandbox --no-default-features --features system-ca-roots

# Default build (bundled Mozilla roots + telemetry)
cargo build -p openshell-sandbox

Compile-time decision rather than runtime because: (1) a distro auditor needs cargo tree proof that bundled roots aren't linked, (2) the supervisor binary is injected into containers with no config surface, and (3) the compile-time guard caught webpki-roots leaking back through transitive dependencies that a runtime flag never would have.

Changes

  • crates/openshell-supervisor-network/Cargo.toml: Make webpki-roots optional behind bundled-ca-roots (default); make rustls-native-certs a regular dependency
  • crates/openshell-supervisor-network/src/l7/tls.rs: Use #[cfg(feature = "bundled-ca-roots")] / #[cfg(not(feature = "bundled-ca-roots"))] to select CA root source
  • crates/openshell-sandbox/Cargo.toml: Forward bundled-ca-roots to supervisor-network; add system-ca-roots convenience alias; set default-features = false on the dependency
  • tasks/rust.toml: Update rust:verify:system-ca-roots to use the alias; fix rust:verify:telemetry-off to pass --features bundled-ca-roots
  • Cargo.toml: Add rustls-native-certs to workspace dependencies; switch reqwest, sqlx, kube to native root variants
  • crates/openshell-sdk/Cargo.toml: Disable oauth2 default features to prevent webpki-roots leaking back through oauth2's rustls-tls default feature
  • .github/workflows/branch-checks.yml: Update step name to use "build mode" terminology
  • architecture/build.md: Document the single-toggle model and alias

Testing

  • mise run rust:verify:telemetry-off passes (telemetry compiled out, CA root guard satisfied)
  • mise run rust:verify:system-ca-roots passes (system CA roots mode compiles, no webpki-roots in dep tree)
  • cargo test -p openshell-supervisor-network passes under default features
  • cargo check --all-features passes (no mutual exclusion conflict)
  • Downstream RPM spec (cargo build --release --bin openshell --bin openshell-gateway) builds without feature flags

Checklist

  • Conventional commit format
  • No bundled Mozilla roots leak into system CA roots builds
  • CLI and gateway use native roots unconditionally
  • CI telemetry-off verification updated
  • cargo --all-features works

maxamillion and others added 2 commits June 30, 2026 17:03
Allow distro builds to use native trust stores for supervisor upstream TLS while keeping bundled Mozilla roots as the default. Avoid bundled root crates in system-ca-roots builds by using native-root TLS features and z3 0.20.

Signed-off-by: Adam Miller <admiller@redhat.com>
The telemetry-off task uses --no-default-features which now disables
bundled-ca-roots in addition to telemetry, triggering the compile_error
guard. Re-enable bundled-ca-roots explicitly so the task verifies only
telemetry compilation.

Signed-off-by: Scott Burdine <sburdine@nvidia.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

Thank you for your interest in contributing to OpenShell, @politerealism.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

  1. Open a Vouch Request discussion.
  2. Describe what you want to change and why.
  3. Write in your own words — do not have an AI generate the request.
  4. A maintainer will comment /vouch if approved.
  5. Once vouched, open a new PR (preferred) or reopen this one after a few minutes.

See CONTRIBUTING.md for details.

@github-actions github-actions Bot closed this Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@politerealism

Copy link
Copy Markdown
Contributor Author

I have read the DCO document and I hereby sign the DCO.

@politerealism

Copy link
Copy Markdown
Contributor Author

recheck

@maxamillion maxamillion reopened this Jul 16, 2026
@maxamillion

Copy link
Copy Markdown
Collaborator

/ok to test 00c9b14

Resolve conflicts in Cargo.lock and crates/openshell-sandbox/Cargo.toml.
Keep default-features = false on openshell-supervisor-network while
adding new middleware dependencies from main.

Signed-off-by: Scott Burdine <sburdine@nvidia.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism

Copy link
Copy Markdown
Contributor Author

@TaylorMutch @derekwaynecarr or others, can I get an Okay to test and review? I'm working on getting the MR for Hummingbird to work with Openshell and I think this PR will get it working.

@maxamillion

Copy link
Copy Markdown
Collaborator

/ok to test b454e32

The bare `oauth2 = "5"` dependency re-enabled default features
(rustls-tls → reqwest/rustls-tls → webpki-roots), defeating the
system-ca-roots feature gate. Mirror the CLI fix: disable defaults
and enable only the `reqwest` feature.

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@maxamillion

Copy link
Copy Markdown
Collaborator

/ok to test dea4d85

@elezar

elezar commented Jul 22, 2026

Copy link
Copy Markdown
Member

@politerealism could you comment on why we elected for a compile-time decision instead of runtime selection?

@politerealism

politerealism commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Yes! @elezar , there are two reasons why it made sense to use compile-time decision instead of runtime selection. If it were runtime, webpki-roots would still be compiled in and linked. A distro packager can't certify a binary that ships it's own trust store alongside the system. The compile_error! guard makes this auditable. Cargo tree proves the dependency is physically absent.
The other reason is the supervisor binary is injected into containers. There's no natural place to put a "use system roots" toggle at runtime without adding confi surface to a security critical component that meant to be minimal.
One more thought is the SDK pulled webpki-roots back in through requests defaults. The completime guard and CI check caught it, but a runtime flag never would have.

Comment thread crates/openshell-supervisor-network/src/l7/tls.rs
Comment thread architecture/build.md Outdated
Replace mutually exclusive bundled-ca-roots / system-ca-roots features
with a single bundled-ca-roots toggle. Disabling it implies system roots
via rustls-native-certs, which is now a regular (non-optional) dependency.
This fixes cargo --all-features and simplifies the distro build interface
from --no-default-features --features system-ca-roots to just
--no-default-features.

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
Comment thread tasks/rust.toml Outdated
description = "Verify system-ca-roots build compiles and excludes bundled Mozilla root crates"
run = [
# Check that the workspace compiles cleanly without bundled CA roots.
"cargo check --workspace --all-targets --no-default-features",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should strictly speaking add --features telemetry here to get the behaviour of the build only excluding the bundled-ca-roots.

Comment thread tasks/rust.toml Outdated
]

["rust:verify:system-ca-roots"]
description = "Verify system-ca-roots build compiles and excludes bundled Mozilla root crates"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We no longer have a system-ca-roots build.

@elezar

elezar commented Jul 29, 2026

Copy link
Copy Markdown
Member

One concern with the current shape is that "system CA mode" is expressed as bare --no-default-features. That makes the CA-root choice entangled with unrelated defaults like telemetry, so package recipes and CI have to remember to manually re-enable every other default feature.

Two options I see here:

Firstly, we introduce named feature-set aliases instead, e.g. default-bundled-ca-roots and default-system-ca-roots, where both include the normal default feature set except for the CA-root provider? Then distro builds can use something like:

cargo build -p openshell-sandbox --no-default-features --features default-system-ca-roots

That still preserves the core guarantee that the system-root artifact is compiled without bundled roots, but makes the build intent explicit and less fragile.

An alternative would be to flip the default. That is to say, by default we build binaries WITHTOUT the bundled-ca-roots feature. We would then explicitly add --features bundled-ca-roots when building a portable sandbox binary. This does require some
initial changes to the CI and build tooling, but the diff doesn't look too large. The bigger concern is whether we're ok with making this breaking change to downstream users that are building openshell from source?

In the longer term, if we want upstream packages to ship both variants, the gateway could select between two prebuilt supervisor artifacts at startup. That should remain artifact selection, not a runtime switch inside one binary, so distro packages that prohibit bundled roots can still ship only the system-root artifact.

@elezar

elezar commented Jul 29, 2026

Copy link
Copy Markdown
Member

One wording/design note: the PR currently describes this as adding bundled-ca-roots / system-ca-roots feature flags, but as implemented only bundled-ca-roots is a Cargo feature. The system-root path is selected by building without bundled-ca-roots.

Could we make that terminology consistent across the PR description/docs/CI labels? I think the clean framing is:

  • bundled-ca-roots: Cargo feature
  • system CA roots: build mode / default when bundled-ca-roots is not enabled

I do not think a feature alias is a blocker here as long as every --no-default-features recipe that wants the normal sandbox behavior consistently re-adds the required default features, especially telemetry. If we see more default features being added to openshell-sandbox, we can revisit adding a named alias such as default-system-ca-roots to avoid that becoming fragile.

Add a system-ca-roots feature alias on openshell-sandbox that includes
all other defaults (telemetry) except bundled-ca-roots, so distro
builds can use --no-default-features --features system-ca-roots without
manually re-adding unrelated defaults. Update the verify CI task to use
the alias and scope checks to the sandbox package. Fix task description
to use "build mode" terminology instead of implying a Cargo feature.

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
Comment on lines +62 to +65
## Convenience alias: all defaults except bundled CA roots. Use
## `--no-default-features --features system-ca-roots` to build a supervisor
## that uses the platform trust store with telemetry intact.
system-ca-roots = ["telemetry"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: move closer to default so that it's simpler to see the diff.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default = ["telemetry", "bundled-ca-roots"]
## Convenience alias for the default sandbox behavior except bundled CA roots.
## Use with `--no-default-features --features system-ca-roots`.
system-ca-roots = ["telemetry"]

telemetry = ["openshell-core/telemetry"]
bundled-ca-roots = ["openshell-supervisor-network/bundled-ca-roots"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

hyper-util = { workspace = true }
miette = { workspace = true }
oauth2 = "5"
oauth2 = { version = "5", default-features = false, features = ["reqwest"] }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: (I haven't checked the diff / PR history) Could you comment on why this change was made?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The local oauth2 manifest confirms its defaults include rustls-tls, which maps to reqwest/rustls-tls; that’s the bundled-root leak."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The oauth2 crate's default features include rustls which pulls in reqwest/rustls-tls-webpki-roots, that drags webpki-roots back into the dependency tree even when bundled-ca-roots is disabled. By setting default-features = false and only enabling reqwest, oauth2 inherits the workspace level configuration which uses rustls-tls-native-roots instead.
Without this fix, cargo tree -i webpki-roots --no-default-features --features system-ca-roots would still find webpki roots in the graph, making the whole feature flag pointless and would also negate my previous fix on it.

@elezar

elezar commented Jul 29, 2026

Copy link
Copy Markdown
Member

/ok-to-test fe2171d

…ar default

Signed-off-by: Quinn Burdine <sburdine@redhat.com>
Signed-off-by: politerealism <burdcat17@gmail.com>
@politerealism
politerealism requested a review from elezar July 30, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants